Skip to content

Fix digital channel reading via high-level read() API#99

Merged
stevevanhooser merged 3 commits intomainfrom
claude/fix-neuropixels-digital-data-8uh77
Apr 10, 2026
Merged

Fix digital channel reading via high-level read() API#99
stevevanhooser merged 3 commits intomainfrom
claude/fix-neuropixels-digital-data-8uh77

Conversation

@stevevanhooser
Copy link
Copy Markdown
Contributor

Summary

This PR fixes the ndr.reader.read() method to properly handle digital input/output channels by routing them through the readchannels_epochsamples code path instead of the abstract readevents_epochsamples method, which was returning empty data.

Key Changes

  • reader.m: Extended the channel type switch statement to include digital channel variants ('digital_input', 'digital_output', 'digital_in', 'digital_out', 'di', 'do') alongside existing analog channel types. This ensures digital channels are processed through the same epoch-based sampling mechanism as analog channels.

  • test.m: Enhanced the Neuropixels GLX reader test to validate digital channel reading:

    • Modified sync channel test data to use a 16-bit ramping pattern (mod((0:nSamples-1), 2^15)) instead of simple 0/1 values, ensuring the full 16-bit digital word is exercised
    • Added comprehensive validation of digital channel reading through the high-level r.read() API, including checks for non-empty data, correct sample count, and bit-perfect preservation of the 16-bit values

Implementation Details

The fix recognizes that digital channels should be treated as sampled data (like analog channels) rather than event-based data. The test enhancement confirms that the full 16-bit digital word is preserved end-to-end through the read pipeline, not collapsed to binary 0/1 values.

https://claude.ai/code/session_0186coedH7nDU1RDPRL3RtPk

claude added 3 commits April 10, 2026 14:42
ndr.reader.read() routed 'digital_in' / 'di' channel types to
readevents_epochsamples (intended for event/marker data), but
ndr.reader.neuropixelsGLX only implements readchannels_epochsamples
for digital_in (the 16-bit NIDQ sync word is regularly sampled at
the same rate as the analog channels). The event path falls through
to the abstract base implementation, which returns [], so callers
like r.read(epoch,'di1','t0',0,'t1',10) got empty data and time.

Route digital_in/digital_out through readchannels_epochsamples in
the high-level read() switch, the same way analog_in is handled.
This preserves the native int16 digital word (it is a 16-bit signal,
not 0/1) and mirrors how intan_rhd already treats its digital lines
in readchannels_epochsamples.

Extend the neuropixelsGLX reader test to exercise the sync channel
through r.read('di1',...) with a 16-bit ramp pattern to guard
against regressions that would collapse the word to 0/1 or re-break
the routing.
In NDR each digital_in channel represents a single bit. Previously
the SpikeGLX reader exposed the entire packed digital word as a
single 'di1' channel, which conflicted with the NDR convention and
made it impossible for callers to ask for an individual line.

Detect digital width from metadata:
  - NIDQ: n_digital_lines = 8 * (niXDBytes1 + niXDBytes2). For the
    user-reported file (niXDBytes1=1) this gives 8 lines (di1..di8).
    A future file with niXDBytes1=2 would yield 16, and adding port1
    via niXDBytes2 extends further. Falls back to 16 * n_dw_chans
    when the byte fields are absent.
  - IMEC: n_digital_lines = 16 * n_sync_chans, so the int16 sync
    word is exposed as di1..di16 (bit 6 is the SMA sync input in
    practice; other bits are available if a recording uses them).

header.m gains two new fields, n_digital_word_cols (number of int16
columns in the file holding digital data, always at the end) and
n_digital_lines (number of single-bit lines exposed). Existing
fields are unchanged so samples2volts and the NIDQ tests keep
working.

getchannelsepoch now emits one digital_in entry per bit
(di1..di_n_digital_lines) instead of a single di1.

readchannels_epochsamples('digital_in', channels, ...) maps each
1-based line index to (DW column, bit position), reads each unique
DW column once via read_samples, and extracts the requested bit
with bitget. Returns int16 0/1 values, one column per requested
line. Out-of-range lines raise
ndr:reader:neuropixelsGLX:DigitalLineOutOfRange.

Tests:
  - +ndr/+test/+reader/+neuropixelsGLX/test.m: writes a counter
    pattern to the IMEC sync column and verifies di1, di8 and di12
    return the correct bit via the high-level r.read() API. Adds a
    NIDQ section that mirrors the user's snsMnMaXaDw=0,0,8,1 +
    niXDBytes1=1 configuration, asserts exactly 8 di channels are
    listed, reads di1 via r.read() (the exact failing call from the
    bug report), and verifies di9 raises DigitalLineOutOfRange.
  - tools/tests/.../TestNeuropixelsGLX.m: testGetChannelsEpoch
    updated to expect 16 digital lines after the neural channels
    instead of a single di1.
The neuropixelsGLX header now stores explicit per-line metadata for
the digital channels it exposes:
  - n_digital_lines     : count of active single-bit lines
  - digital_line_col    : 0-based DW column offset for each line
  - digital_line_bit    : 0-based bit position within that column
  - digital_line_label  : underlying SpikeGLX line label, e.g.
                          'XD0'..'XD7' for port-0, 'XD1.<k>' for
                          port-1, 'SY<col>.<bit>' for IMEC sync.

NIDQ line counts come from niXDBytes1/niXDBytes2 (8 active lines per
saved byte). NI hardware only enables digital input in whole-byte
chunks, so every bit within a captured byte is electrically active
even when the user only physically wired some of them. niXDChans1/2
remains informational and is not used to gate which lines are
exposed. IMEC streams have no per-bit configuration, so all 16 bits
of each sync int16 column are exposed.

readchannels_epochsamples no longer recomputes (col, bit) on the
fly; it just looks up info.digital_line_col / digital_line_bit by
the requested 1-based di index. The bit-extraction loop, the unique
column grouping, and the read_samples call sites are unchanged, so
behavior for the user's file (8 di channels mapping to bits 0..7 of
the only DW column) is identical.
Comment thread +ndr/reader.m
% readchannels_epochsamples
case {'analog_input','analog_output','analog_in','analog_out','ai','ao'},
case {'analog_input','analog_output','analog_in','analog_out','ai','ao', ...
'digital_input','digital_output','digital_in','digital_out','di','do'},
@github-actions
Copy link
Copy Markdown
Contributor

Test Results

123 tests  ±0   123 ✅ ±0   5s ⏱️ ±0s
 14 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit c17191a. ± Comparison against base commit 6548b71.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

❌ Patch coverage is 34.16149% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.95%. Comparing base (6548b71) to head (c17191a).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
+ndr/+test/+reader/+neuropixelsGLX/test.m 0.00% 87 Missing ⚠️
+ndr/+format/+neuropixelsGLX/header.m 68.62% 16 Missing ⚠️
+ndr/+reader/neuropixelsGLX.m 85.71% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
- Coverage   31.98%   31.95%   -0.03%     
==========================================
  Files          92       92              
  Lines        4656     4810     +154     
==========================================
+ Hits         1489     1537      +48     
- Misses       3167     3273     +106     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stevevanhooser stevevanhooser merged commit 5c5c19e into main Apr 10, 2026
5 checks passed
@stevevanhooser stevevanhooser deleted the claude/fix-neuropixels-digital-data-8uh77 branch April 10, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants